Upgraded trust-dns-resolver (#92) #105
Conversation
|
Hey @bigtoast, thanks for the PR! I can't right now, but I'll take a look at it tonight (GMT+1) |
src/os/shared.rs
Outdated
| let mut runtime = Runtime::new()?; | ||
| let resolver = runtime | ||
| .block_on(dns::Resolver::new(runtime.handle().clone())) | ||
| .unwrap(); |
There was a problem hiding this comment.
We can remove this unwrap() and use a question mark here
There was a problem hiding this comment.
I made this change but for my own edification, can you expand on the reasoning for it? I am certainly new to the idioms and best practices for Rust.
There was a problem hiding this comment.
Sure, I will try my best!
unwrap() will try to access the value inside the Result and will panic in case of error. The question mark will desugar to a match statement that, in case of error, will return early from the function. Another nice thing from the ? is that it will try to convert the unwrapped error into the error type in the return value of the signature, in case the latter implements From for that error.
You can read more here and here.
unwrap is usually frowned upon because it does not allow handling the error gracefully.
|
@ebroto Thanks for the review. I made the requested changes. |
|
Thanks for your fast response! I think that everything has been fixed except for the permissions of the EDIT: It seems you did that faster than I wrote it :) Just let the CI pass and then we can merge. |
|
Thanks for your contribution, feel free to take another one if you wish :) |
This is my first pull request in Rust so feel free to be brutal. All the tests pass and I am able to turn on and off dns resolution on my machine (Fedora 31). Thanks for labelling the issue as a good one to start on. I like this project and would like to help out more as a good way to get some Rust experience. I have a lot of async programming experience but Rust's memory model is a bit out of left field for me.
(edited to add this)
Closes #92